Fix round rect with negative values#2303
Conversation
ankith26
left a comment
ankith26
left a comment
There was a problem hiding this comment.
Hello, thanks for the PR.
As I'm not experienced in these parts of the docs, I'm stuggling to understand even the smallest diffs 😅
I left some questions as reviews
| rect->h = -rect->h; | ||
| } | ||
| // Ignore negative values | ||
| radius = MAX(radius, 0); |
There was a problem hiding this comment.
Instead of simply ignoring negative values here, we could raise an error if this is negative instead
There was a problem hiding this comment.
That could be another solution indeed
There was a problem hiding this comment.
silently clamping it to 0 vs raising an error
I'm not sure which is preferable when using the code. I tend to be against silent things. Make is explicit, not implicit.
|
|
||
| if (width > rect->w / 2 || width > rect->h / 2) { | ||
| width = MAX(rect->w / 2, rect->h / 2); | ||
| width = 0; |
There was a problem hiding this comment.
I'm not sure how this change is helping fix the issue, it looks like it will change behaviour against what is documented
| if (!bottom_left) | ||
| bottom_left = radius; | ||
| if (bottom_right < 0) | ||
| if (!bottom_right) |
There was a problem hiding this comment.
As I understand from the docs
if any of these values are 0, the corner should have no rounded, and if the value is negative (default) it should "inherit" radius. This fix seems to be breaking whatever is documented in my understand, correct me if I'm missing something
There was a problem hiding this comment.
Wait a second, you are probably right. Gonna check tomorrow.
There was a problem hiding this comment.
He is right. This way a border_top_right_radius radius of 0 would inherit the value of border_radius whereas before it would set that corner to have no rounding.
I could imagine this breaking usage if somebody used the popular GUI button pattern of three rounded corners (set via border_radius and one at right angles set with border_top_right_radius.
MyreMylar
left a comment
MyreMylar
left a comment
There was a problem hiding this comment.
Looks like this needs more work to finish off.
5 participants
Fix #2285.
During this I discovered drawing round rect and circle quadrant have another issue not related to this one, but that would require another PR.